Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configure /config/config.R #17

Merged
merged 8 commits into from
Mar 25, 2024
Merged

Configure /config/config.R #17

merged 8 commits into from
Mar 25, 2024

Conversation

Sweetdevil144
Copy link
Contributor

Suggestions initially added by @infotroph for the pecan-status-board to directly fetch our local dockerised server (http://pecan.localhost/) instead of http://141.142.216.168/ which was a virtual machine at NCSA that’s apparently down.

Sweetdevil144 and others added 2 commits March 18, 2024 16:38
Suggestions added by Chris for the status-board to directly fetch our local server over docker instead of `141.142.216.168` which was a virtual machine at NCSA that’s apparently down
Copy link
Member

@mdietze mdietze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than swapping one hard-coded URL for a different hard-coded URL, could we make the URL a variable that we define once and then use everywhere?

@Sweetdevil144
Copy link
Contributor Author

Rather than swapping one hard-coded URL for a different hard-coded URL, could we make the URL a variable that we define once and then use everywhere?

Yes. I have several ideas in my mind for that.

  • We can use the server link by adding it to our .env file and then using the same everywhere.
  • We can ask the user to specify the host by doing something like this:
library("statusboard")
statusboard::run_app() --host {host_url}

We will the https://pecan.localhost/ as the default host_url in scenarios where no such host is defined

  • Take user input explicitly, but this time confirming host by user everytime we run our app. Something like this :
library("statusboard")
statusboard::run_app()
> Enter your host_url (default : https://pecan.localhost) : 

Which one do you find the most suitable for this?

@mdietze
Copy link
Member

mdietze commented Mar 18, 2024

I don't think you want to rely on host_url being set interactively, that could put limitations on where/how the app is deployed. For this PR I think it's fine to just define the variable host_url in one place that's clearly documented, either in the script itself or in a simple config file. I'm also fine with https://pecan.localhost being the default.

@Sweetdevil144
Copy link
Contributor Author

Changes made :

  • Created config/config.R which contains our default host_url for server's connection. All future configurations too may be added in this file in future changes.

R/fct_workflow_status.R Outdated Show resolved Hide resolved
R/fct_workflow_status.R Outdated Show resolved Hide resolved
inst/ed2-test.R Outdated Show resolved Hide resolved
inst/maespa-test.R Outdated Show resolved Hide resolved
inst/run-test-list.R Outdated Show resolved Hide resolved
inst/sipnet-test.R Outdated Show resolved Hide resolved
@Sweetdevil144 Sweetdevil144 changed the title Update server URLs in code Configure /config/config.R Mar 19, 2024
@Sweetdevil144
Copy link
Contributor Author

Hey @mdietze . Ready to be merged !

@mdietze mdietze merged commit 931390f into PecanProject:main Mar 25, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants